Conversation
seanabrahams
force-pushed
the
active-record-5.2
branch
from
37a22fd to
5b6fc74
Compare
seanabrahams
force-pushed
the
active-record-5.2
branch
from
5b6fc74 to
f81f515
Compare
|
Any timetable for 5.2 compatibility? |
|
Is it possible to get this merged in? We are on Rails 5.2.3, and had broken tests (totally unrelated to any encrypted columns). Using @seanabrahams patch here fixed all the issues for us. Thank you. |
| if ::ActiveRecord::VERSION::STRING >= "5.2" | ||
| # This is needed support attribute_was before a record has | ||
| # been saved | ||
| set_attribute_was(attr, __send__(attr)) if value != __send__(attr) |
There was a problem hiding this comment.
Works well with ActiveRecord 5.2.
However, in ActiveRecord 6.0.0, private method set_attribute_was was removed. rails/rails@6b0a9de
There was a problem hiding this comment.
I'm not sure how we should proceed with a fix that only works in a single release. My first thought was to allow this to merge, and for Rails 6 we create a new major release. That will also give us the opportunity to finally deprecate some old code and potentially do some major cleanup of the test suite.
Any thoughts on how we can move forward? Is my suggestion the best course or are there better ideas?
There was a problem hiding this comment.
Just update the pull request with the code snippet bellow
#337 (comment)
|
Will this ever merge? |
|
@seanabrahams any news about this getting merged? Thanks. |
|
@saghaulor @shuber @sbfaulkner @billymonk Thanks so much for maintain Please let us know how can we help to get this PR merged, thank you! |
|
For ActiveRecord > 6.0.0 is no more possible use lib/attr_encrypted/adapters/active_record.rb line 86 #set_attribute_was(attr, __send__(attr)) if value != __send__(attr)
if ActiveRecord.version <= Gem::Version.new('5.3.0')
set_attribute_was(attr, __send__(attr)) if value != __send__(attr)
else
unless #{attr}_changed?
@attributes[attr] = ActiveModel::Attribute.from_user(attr, __send__(attr), ActsAsTaggableOn::Taggable::TagListType.new)
end
end |
|
please see #379 |
11 participants
I'm not happy with this solution as it feels like a hack, but the tests pass and it gets the ball rolling.